Skip to content

Enabling/fixing debug/undebug feature for quagga/frr#254

Open
rodnymolina wants to merge 1 commit intosonic-net:masterfrom
rodnymolina:debug_undebug_fixes
Open

Enabling/fixing debug/undebug feature for quagga/frr#254
rodnymolina wants to merge 1 commit intosonic-net:masterfrom
rodnymolina:debug_undebug_fixes

Conversation

@rodnymolina
Copy link
Contributor

Note: PR depends on PR/1714

With this PR I'm taking care of the following issues:

   * Fixed debug/undebug cli commands for bgp and zebra processes.
   * Added a new cli command to activate/deactivate the generation of debuging info.
   * Added two separated command structures for Quagga and FRR routing-stacks -- notice that they don't fully match.
   * Also notice that FRR's approach doesn't require the explicit definition of all subcommands, as we're passing this information to FRR's vtysh daemon.

New debug/undebug stanzas now look like this:

Quagga example

admin@lnos-x1-a-csw01:~$ debug ?
Usage: debug [OPTIONS] COMMAND [ARGS]...

SONiC command line - 'debug' command

Options:
  -?, -h, --help  Show this message and exit.

Commands:
  bgp      debug bgp events
  disable  disable debugging for routing events
  enable   enable debugging for routing events
  zebra    debug bgp events

admin@lnos-x1-a-csw01:~$ debug bgp ?
Usage: debug bgp [OPTIONS] COMMAND [ARGS]...

debug bgp events

Options:
  -?, -h, --help  Show this message and exit.

Commands:
  as4         debug bgp AS4 actions
  events      debug bgp events
  filters     debug bgp filters
  fsm         debug bgp fsm
  keepalives  debug bgp keepalives
  updates     debug bgp updates
  zebra       debug bgp zebra messages

admin@lnos-x1-a-csw01:~$ debug zebra ?
Usage: debug zebra [OPTIONS] COMMAND [ARGS]...

debug bgp events

Options:
  -?, -h, --help  Show this message and exit.

Commands:
  events  debug zebra events
  fpm     debug zebra fpm events
  kernel  debug zebra's kernel-interface events
  packet  debug zebra packets
  rib     debug zebra RIB events

FRR example

admin@lnos-x1-a-csw02:~$ debug ?
Usage: debug [OPTIONS] COMMAND [ARGS]...

  SONiC debugging commands for routing events

Options:
  -?, -h, --help  Show this message and exit.

Commands:
  bgp      Debug BGP information
  disable  Disable debugging for routing events
  enable   Enable debugging for routing events
  zebra    Debug Zebra information

admin@lnos-x1-a-csw02:~$ debug bgp ?
Command: sudo vtysh -c "debug bgp ?"
  allow-martians   BGP allow martian next hops
  as4              BGP AS4 actions
  bestpath         BGP bestpath
  keepalives       BGP keepalives
  neighbor-events  BGP Neighbor Events
  nht              BGP nexthop tracking events
  update-groups    BGP update-groups
  updates          BGP updates
  zebra            BGP Zebra messages

admin@lnos-x1-a-csw02:~$ debug zebra ?
Command: sudo vtysh -c "debug zebra ?"
  events       Debug option set for zebra events
  fpm          Debug zebra FPM events
  kernel       Debug option set for zebra between kernel interface
  mpls         Debug option set for zebra MPLS LSPs
  nht          Debug option set for zebra next hop tracking
  packet       Debug option set for zebra packet
  pseudowires  Debug option set for zebra pseudowires
  rib          Debug RIB events

UT's
====

admin@lnos-x1-a-csw02:~$ debug enable
Command: sudo vtysh -c "configure terminal" -c "log syslog debugging"

admin@lnos-x1-a-csw02:~$ debug bgp ?
Command: sudo vtysh -c "debug bgp ?"
  allow-martians   BGP allow martian next hops
  as4              BGP AS4 actions
  bestpath         BGP bestpath
  keepalives       BGP keepalives
  neighbor-events  BGP Neighbor Events
  nht              BGP nexthop tracking events
  update-groups    BGP update-groups
  updates          BGP updates
  zebra            BGP Zebra messages

admin@lnos-x1-a-csw02:~$ debug bgp keepalive
Command: sudo vtysh -c "debug bgp keepalive"
BGP keepalives debugging is on

admin@lnos-x1-a-csw02:~$ sudo tail -f /var/log/syslog  [ destination log file is as per /etc/rsyslog.d/00-sonic.conf ]
...
May  9 23:51:48.525961 lnos-x1-a-csw02 DEBUG bgpd[65]: fc00:2:2::2 sending KEEPALIVE
May  9 23:51:48.526019 lnos-x1-a-csw02 DEBUG bgpd[65]: 10.2.2.2 sending KEEPALIVE
May  9 23:51:48.526054 lnos-x1-a-csw02 DEBUG bgpd[65]: fc00:2:2::2 KEEPALIVE rcvd
May  9 23:51:48.526123 lnos-x1-a-csw02 DEBUG bgpd[65]: 10.2.2.2 KEEPALIVE rcvd
...

admin@lnos-x1-a-csw02:~$ undebug bgp keepalive
Command: sudo vtysh -c "no debug bgp keepalive"
BGP keepalives debugging is off

With this PR I'm taking care of the following issues:

       * Fixed debug/undebug cli commands for bgp and zebra processes.
       * Added a new cli command to activate/deactivate the generation of debuging info.
       * Added two separated command structures for Quagga and FRR routing-stacks -- notice that they don't fully match.
       * Also notice that FRR's approach doesn't require the explicit definition of all subcommands, as we're passing this information to FRR's vtysh daemon.

New debug/undebug stanzas now look like this:

Quagga example
==============

    admin@lnos-x1-a-csw01:~$ debug ?
    Usage: debug [OPTIONS] COMMAND [ARGS]...

    SONiC command line - 'debug' command

    Options:
      -?, -h, --help  Show this message and exit.

    Commands:
      bgp      debug bgp events
      disable  disable debugging for routing events
      enable   enable debugging for routing events
      zebra    debug bgp events

    admin@lnos-x1-a-csw01:~$ debug bgp ?
    Usage: debug bgp [OPTIONS] COMMAND [ARGS]...

    debug bgp events

    Options:
      -?, -h, --help  Show this message and exit.

    Commands:
      as4         debug bgp AS4 actions
      events      debug bgp events
      filters     debug bgp filters
      fsm         debug bgp fsm
      keepalives  debug bgp keepalives
      updates     debug bgp updates
      zebra       debug bgp zebra messages

    admin@lnos-x1-a-csw01:~$ debug zebra ?
    Usage: debug zebra [OPTIONS] COMMAND [ARGS]...

    debug bgp events

    Options:
      -?, -h, --help  Show this message and exit.

    Commands:
      events  debug zebra events
      fpm     debug zebra fpm events
      kernel  debug zebra's kernel-interface events
      packet  debug zebra packets
      rib     debug zebra RIB events

FRR example
============

    admin@lnos-x1-a-csw02:~$ debug ?
    Usage: debug [OPTIONS] COMMAND [ARGS]...

      SONiC debugging commands for routing events

    Options:
      -?, -h, --help  Show this message and exit.

    Commands:
      bgp      Debug BGP information
      disable  Disable debugging for routing events
      enable   Enable debugging for routing events
      zebra    Debug Zebra information

    admin@lnos-x1-a-csw02:~$ debug bgp ?
    Command: sudo vtysh -c "debug bgp ?"
      allow-martians   BGP allow martian next hops
      as4              BGP AS4 actions
      bestpath         BGP bestpath
      keepalives       BGP keepalives
      neighbor-events  BGP Neighbor Events
      nht              BGP nexthop tracking events
      update-groups    BGP update-groups
      updates          BGP updates
      zebra            BGP Zebra messages

    admin@lnos-x1-a-csw02:~$ debug zebra ?
    Command: sudo vtysh -c "debug zebra ?"
      events       Debug option set for zebra events
      fpm          Debug zebra FPM events
      kernel       Debug option set for zebra between kernel interface
      mpls         Debug option set for zebra MPLS LSPs
      nht          Debug option set for zebra next hop tracking
      packet       Debug option set for zebra packet
      pseudowires  Debug option set for zebra pseudowires
      rib          Debug RIB events

    UT's
    ====

    admin@lnos-x1-a-csw02:~$ debug enable
    Command: sudo vtysh -c "configure terminal" -c "log syslog debugging"

    admin@lnos-x1-a-csw02:~$ debug bgp ?
    Command: sudo vtysh -c "debug bgp ?"
      allow-martians   BGP allow martian next hops
      as4              BGP AS4 actions
      bestpath         BGP bestpath
      keepalives       BGP keepalives
      neighbor-events  BGP Neighbor Events
      nht              BGP nexthop tracking events
      update-groups    BGP update-groups
      updates          BGP updates
      zebra            BGP Zebra messages

    admin@lnos-x1-a-csw02:~$ debug bgp keepalive
    Command: sudo vtysh -c "debug bgp keepalive"
    BGP keepalives debugging is on

    admin@lnos-x1-a-csw02:~$ sudo tail -f /var/log/syslog  [ destination log file is as per /etc/rsyslog.d/00-sonic.conf ]
    ...
    May  9 23:51:48.525961 lnos-x1-a-csw02 DEBUG bgpd[65]: fc00:2:2::2 sending KEEPALIVE
    May  9 23:51:48.526019 lnos-x1-a-csw02 DEBUG bgpd[65]: 10.2.2.2 sending KEEPALIVE
    May  9 23:51:48.526054 lnos-x1-a-csw02 DEBUG bgpd[65]: fc00:2:2::2 KEEPALIVE rcvd
    May  9 23:51:48.526123 lnos-x1-a-csw02 DEBUG bgpd[65]: 10.2.2.2 KEEPALIVE rcvd
    ...

    admin@lnos-x1-a-csw02:~$ undebug bgp keepalive
    Command: sudo vtysh -c "no debug bgp keepalive"
    BGP keepalives debugging is off
Copy link
Contributor

@nikos-github nikos-github left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments in #222 which you haven't followed, still apply.

@rodnymolina
Copy link
Contributor Author

I did took #222 into account, and that's one of the reasons i removed all FRR related debug commands. Only Quagga debug commands are exposed, as this one is not using our knob to pass cli-commands transparently to the backend.

Concerning the 'undebug' vs 'no debug', we can discuss if we want to migrate in the future towards the 'no debug' stanza, but what we have today is 'undebug', and this one is broken, so this PR is simply fixing what is already out there.

Copy link
Contributor

@nikos-github nikos-github left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If undebug is broken, then best not to fix it since it is removed in FRR.

I recommend following the same approach you have for FRR, for quagga as well. It has made the code and functionality much better.

Ultimately sonic MS team is to bless this.

@rodnymolina
Copy link
Contributor Author

Right, my goal is to extend my FRR changes to Quagga too, but that will require more changes in more repos. This current patch is already involving changes in three different repos. Quagga changes can come later.

We should fix 'debug/undebug' cli now. If the cli implemented by the backend services change, we can later decide to update our front-end cli for end-to-end consistency, or simply leave it the way it is today as we wouldn't be breaking any functionality thanks to Click wrappers.

mihirpat1 pushed a commit to mihirpat1/sonic-utilities that referenced this pull request Sep 15, 2023
Description
Fixed typo FW_AUTO_ERR_UKNOWN --> FW_AUTO_ERR_UNKNOWN

Motivation and Context
This is a typo and in the interest of a readable code base should not remain. There are also very few references to this as of now so changing it now before more references are added is a priority.

How Has This Been Tested?
Unit tests passing
dgsudharsan pushed a commit to dgsudharsan/sonic-utilities that referenced this pull request Nov 18, 2025
…et#254)

***Important***: Please review each commit (including commit message) individually. Looking at the patch-set as a whole may cause confusion.

#### What I did

Generic Configuration Updater is extremely slow, using the python profiler it was possible to determine the worst offenders where changes could be made without affecting the overall algorithm and HLD design documentation.

Brief overview of changes:
* Prevent copy.deepcopy() calls where possible
* Don't run validation twice back to back
* Move configdb path <> xpath conversion logic to sonic-yang-mgmt where it belongs and enhance it to support schema conversion (not just data) and add caching.
* Sort table keys by the number of schema backlinks and must statements for the node to try better guess the right order of the patches to generate rather than doing it in alphabetical order which is likely to cause validation failures.
* Add ability to Group patches together in some commits where its known they will not cause issues, these are things like grouping parameter updates under the same key.
* When applying changes, do not re-read the configuration from redis twice between each applied patch (this is **extremely** slow, and actually hid a race condition). We are mutating the configuration and a lock is held so we know the expected before and after. There is still a final validation to ensure something didn't go sideways.

Dependencies:
 failure_prs.log sonic-yang-mgmt enhancements: sonic-net/sonic-buildimage#22254
 failure_prs.log sonic-yang-mgmt parse uses/grouping: sonic-net/sonic-buildimage#21907
 failure_prs.log sonic-utilities rely on sonic-yang-mgmt uses/grouping handling: sonic-net#3814

Stats below ... (stats need both this and the sonic-utilities PR to be relevant)...

<ins>**Original Performance:**</ins>
Dry Run:
```
time sudo config replace -d ./config_db.json
...
real	2m51.588s
user	2m23.777s
sys	0m25.300s
```

Full:
```
time sudo config replace ./config_db.json
...
real	14m53.772s
user	12m2.376s
sys	2m8.908s
```

<ins>**With Patch**:</ins>
Dry Run:
```
time sudo config replace -d ./config_db.json
...
real	0m59.602s
user	0m56.434s
sys	0m2.110s
```

Full:
```
time sudo config replace ./config_db.json
...
real	1m54.303s
user	0m58.482s
sys	0m2.545s
```

So that's roughly 3x improvement for dry-run, and 7.5x improvement for full commit. There is room for improvement on the full commit due to a `sleep(1)` being used between each patch because of a race condition found in the prior code (that was hidden due to a costly sanity check that has been removed).

#### How I did it

Profiling via cProfiler

#### How to verify it

Run sonic-utilities test cases, they still pass

#### Previous command output (if the output of a command-line utility has changed)

N/A

#### New command output (if the output of a command-line utility has changed)

N/A

Fixes sonic-net/sonic-buildimage#22372
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants